Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

.github/workflows/pr.yaml (1)

180-196: Add SONAR_HOST_URL to quality gate action for consistency and reliability.

While the sonarqube-quality-gate-action can infer the SonarQube host URL from the scan report (report-task.txt), it's recommended to explicitly provide SONAR_HOST_URL for both the scan and quality gate steps, especially with self-hosted SonarQube instances. This ensures the quality gate can reliably contact the correct server without depending on the report file.

Proposed fix

       - uses: SonarSource/sonarqube-quality-gate-action@v1
         timeout-minutes: 5
         env:
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
+          SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }}

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

sonarqube/run-sonar-analysis.sh (3)

104-104: Consider sanitizing branch names to prevent command injection.

The $branch variable is read from a file and used directly in git clone --branch "$branch". While unlikely in a controlled environment, a maliciously crafted branch name in sonar-branches.txt could potentially cause issues. Consider validating branch names match expected patterns.

🔧 Optional: Add branch name validation

   # Trim whitespace and add to array
   branch=$(echo "$branch" | xargs)
+  # Validate branch name contains only safe characters
+  if [[ ! "$branch" =~ ^[a-zA-Z0-9._/-]+$ ]]; then
+    echo "WARNING: Skipping invalid branch name: $branch"
+    continue
+  fi
   branches+=("$branch")

---

159-163: Improve failure classification logic.

The current logic checks for "Build failed" string in the build log, but if the build log doesn't exist or the string isn't found, it defaults to "sonar analysis failed". This could misclassify failures. Consider checking log existence and using more robust detection.

🔧 More robust failure detection

-    if grep -q "Build failed" /tmp/build_${safe_branch_name}.log 2>/dev/null; then
-      failed_branches+=("$branch (build failed)")
-    else
-      failed_branches+=("$branch (sonar analysis failed)")
-    fi
+    if [ -f /tmp/build_${safe_branch_name}.log ] && grep -q "Build failed" /tmp/build_${safe_branch_name}.log; then
+      failed_branches+=("$branch (build failed)")
+    elif [ -f /tmp/sonar_${safe_branch_name}.log ] && grep -q "SonarQube analysis failed" /tmp/sonar_${safe_branch_name}.log; then
+      failed_branches+=("$branch (sonar analysis failed)")
+    else
+      failed_branches+=("$branch (unknown failure)")
+    fi

---

49-57: Consider preserving logs on failure before cleanup.

The trap cleanup EXIT removes $WORK_DIR on exit, but the logs are stored in /tmp/ outside this directory, so they're preserved. However, it might be helpful to explicitly mention this or provide an option to preserve the working directory for debugging failed branches.

sonarqube/README.md (1)

25-33: Consider documenting the optional SONAR_HOST_URL variable.

The script supports a custom SonarQube server via SONAR_HOST_URL, but the README only mentions SONAR_TOKEN. Adding this information would help users with self-hosted SonarQube instances.

📝 Suggested addition

 ### 2. Set Required Environment Variable

 The script requires authentication:

 ```bash
 export SONAR_TOKEN="your_token_here"
+
+# Optional: For self-hosted SonarQube (defaults to SonarCloud)
+export SONAR_HOST_URL="https://your-sonarqube-server.com"

Note: Contact the cuOpt team for token details.

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used**: Path: .coderabbit.yaml

**Review profile**: CHILL

**Plan**: Pro

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between f2b8381f81cf1031649da69a5d55161ce35305f8 and ac0a4dbe786c24cbc000ba5b1d32c5dd019af7d7.

</details>

<details>
<summary>📒 Files selected for processing (5)</summary>

* `CONTRIBUTING.md`
* `sonar-project.properties`
* `sonarqube/README.md`
* `sonarqube/run-sonar-analysis.sh`
* `sonarqube/sonar-branches.txt`

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (2)</summary>

* sonarqube/sonar-branches.txt
* sonar-project.properties

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 Shellcheck (0.11.0)</summary>

<details>
<summary>sonarqube/run-sonar-analysis.sh</summary>

[warning] 111-111: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)</summary>

* GitHub Check: wheel-build-cuopt-sh-client / 13.1.0, 3.10, amd64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, amd64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, amd64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, amd64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
* GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8

</details>

<details>
<summary>🔇 Additional comments (5)</summary><blockquote>

<details>
<summary>sonarqube/run-sonar-analysis.sh (3)</summary><blockquote>

`1-8`: **LGTM on script initialization.**

Proper license header and sensible defaults for script directory detection and timestamped working directory.

---

`9-28`: **Good security practice for token handling.**

Requiring `SONAR_TOKEN` via environment variable (rather than hardcoding or accepting as argument) is the correct approach. The helpful error message with instructions is a nice touch.

---

`184-219`: **LGTM on the summary and exit logic.**

Clear summary output with success/failure tracking and appropriate exit codes. The cleanup via trap ensures temporary directories are removed even on failures.

</blockquote></details>
<details>
<summary>sonarqube/README.md (1)</summary><blockquote>

`1-60`: **Good documentation coverage.**

The README provides clear instructions covering the essential workflow: configuring branches, setting credentials, and running the script. The script behavior section accurately reflects what the script does.

</blockquote></details>
<details>
<summary>CONTRIBUTING.md (1)</summary><blockquote>

`139-139`: **The conda environment file reference is correct and complete.**

The CUDA version update from 130 to 131 is properly implemented. Both required environment files (`all_cuda-131_arch-aarch64.yaml` and `all_cuda-131_arch-x86_64.yaml`) exist in the repository, and all references to the old version have been removed. The change is consistently applied across CONTRIBUTING.md and the sonarqube integration scripts.

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

sonarqube/run-sonar-analysis.sh (2)

92-92: Log files in /tmp are not cleaned up.

Log files (/tmp/clone_*.log, /tmp/conda_create_*.log, /tmp/build_*.log, /tmp/sonar_*.log) are written outside WORK_DIR and won't be cleaned up by the exit trap. Over time, these can accumulate.

Proposed fix: Write logs to WORK_DIR

-  if ! git clone --single-branch --branch "$branch" --depth 1 "$REPO_URL" "$clone_dir" 2>&1 | tee /tmp/clone_${safe_branch_name}.log; then
+  if ! git clone --single-branch --branch "$branch" --depth 1 "$REPO_URL" "$clone_dir" 2>&1 | tee "$WORK_DIR/clone_${safe_branch_name}.log"; then

Apply similar changes to lines 112, 132, and 142 to use $WORK_DIR/ prefix for all log files.

---

139-145: Token handling in bash -c could break with special characters.

If SONAR_TOKEN contains single quotes or other shell metacharacters, the string interpolation into the bash -c command could cause syntax errors or unexpected behavior.

Proposed fix: Export token and reference in subshell

+  # Export token for subshell access
+  export SONAR_TOKEN
+
   if ! bash -c "
     set -e
     source \$(conda info --base)/etc/profile.d/conda.sh
     conda activate $conda_env_name
     ...
     # Run SonarQube analysis
     echo 'Running SonarQube analysis...'
     if ! sonar-scanner \
-      -Dsonar.token='$SONAR_TOKEN' \
+      -Dsonar.token=\"\$SONAR_TOKEN\" \
       -Dsonar.branch.name='$branch' \
       2>&1 | tee /tmp/sonar_${safe_branch_name}.log; then

By exporting the variable and referencing it with \$SONAR_TOKEN (escaped to defer expansion to the subshell), the token is passed safely without string interpolation issues.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

sonarqube/run-sonar-analysis.sh (1)

1-4: Consider adding set -euo pipefail for safer script execution.

Adding strict mode helps catch unset variables and ensures pipeline failures are detected. This is particularly relevant given the use of | tee patterns later in the script.

Suggested improvement

 #!/bin/bash
 # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 # SPDX-License-Identifier: Apache-2.0
+set -euo pipefail

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

sonarqube/run-sonar-analysis.sh (2)

108-114: Consider adding a fallback from mamba to conda.

The script uses mamba directly, which may not be installed in all environments. Consider falling back to conda if mamba is unavailable.

Proposed fix

+  # Use mamba if available, otherwise fall back to conda
+  if command -v mamba &> /dev/null; then
+    CONDA_CMD="mamba"
+  else
+    CONDA_CMD="conda"
+  fi
+
   # Create conda environment
-  mamba env create -n "$conda_env_name" -f "$CONDA_ENV_FILE" 2>&1 | tee /tmp/conda_create_${safe_branch_name}.log
+  $CONDA_CMD env create -n "$conda_env_name" -f "$CONDA_ENV_FILE" 2>&1 | tee /tmp/conda_create_${safe_branch_name}.log

---

119-147: Complex subshell with string interpolation is fragile.

The bash -c with embedded script string works but is difficult to maintain and debug. The mix of escaped and unescaped variables ($conda_env_name vs \$(python --version)) is error-prone.

Consider extracting this into a separate helper script or using a heredoc with proper quoting for better maintainability.

Alternative approach using a heredoc

  # Write a temporary script for the subshell
  cat > /tmp/build_and_analyze_${safe_branch_name}.sh << 'INNER_SCRIPT'
#!/bin/bash
set -e
source $(conda info --base)/etc/profile.d/conda.sh
conda activate "$1"

echo "Conda environment activated: $1"
echo "Python version: $(python --version)"

echo 'Building project...'
./build.sh 2>&1 | tee "$2"
if [ ${PIPESTATUS[0]} -ne 0 ]; then
  echo 'Build failed'
  exit 1
fi

echo 'Running SonarQube analysis...'
sonar-scanner -Dsonar.branch.name="$3" 2>&1 | tee "$4"
if [ ${PIPESTATUS[0]} -ne 0 ]; then
  echo 'SonarQube analysis failed'
  exit 1
fi

echo 'Build and analysis completed successfully'
INNER_SCRIPT

  chmod +x /tmp/build_and_analyze_${safe_branch_name}.sh
  if ! /tmp/build_and_analyze_${safe_branch_name}.sh \
      "$conda_env_name" \
      "/tmp/build_${safe_branch_name}.log" \
      "$branch" \
      "/tmp/sonar_${safe_branch_name}.log"; then
    # ... error handling
  fi

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

sonarqube/run-sonar-analysis.sh (1)

7-7: Make WORK_DIR collision‑safe for concurrent runs.

Timestamp‑only paths can collide if the script is launched twice within the same second, which can mix logs and cleanup. Prefer mktemp -d (or add $$) for uniqueness.

💡 Suggested change

-WORK_DIR="/tmp/sonar-analysis-$(date +%Y%m%d-%H%M%S)"
+WORK_DIR="$(mktemp -d -t sonar-analysis.XXXXXX)"

Please verify mktemp is available on the target hosts (most Linux distros ship it by default).

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}